add foldMapA as an alternative to foldMapM that only requires an Applicative rather than a monad #3130
Conversation
|
This could probably be done in a binary-compatible way by making the current implementation package private and adding the new one as an overload. I agree with @LukaJCB that it probably makes more sense to add a new It seems like the bigger issue with this implementation is short circuiting. For example, this will hang forever with the import cats.implicits._
Stream.iterate(0)(_ + 1).foldMapM(b => if (b > 10) Left(b) else Right(b))I think an implementation like this should be fine both in terms of stack safety and short-circuiting: def foldMapA[G[_], A, B](fa: F[A])(f: A => G[B])(implicit G: Applicative[G], B: Monoid[B]): G[B] =
foldRight(fa, Eval.now(G.pure(B.empty)))((a, egb) =>
G.map2Eval(f(a), egb)(B.combine)
).valueFor example: scala> import cats.implicits._
import cats.implicits._
scala> Stream.iterate(0)(_ + 1).foldMapA(b => if (b > 10) Left(b) else Right(b))
res0: scala.util.Either[Int,Int] = Left(11)
scala> Stream.iterate(0)(_ + 1).foldMapA(b => if (b > 1000000) Left(b) else Right(b))
res1: scala.util.Either[Int,Int] = Left(1000001) |
|
Hey there, Thanks for the review! I didn't know about map2Eval, that's very interesting. I've now added |
Codecov Report
@@ Coverage Diff @@
## master #3130 +/- ##
=========================================
+ Coverage 93.19% 93.2% +<.01%
=========================================
Files 376 376
Lines 7323 7324 +1
Branches 190 186 -4
=========================================
+ Hits 6825 6826 +1
Misses 498 498
Continue to review full report at Codecov.
|
| /** | ||
| * Equivalent to foldMapM. | ||
| * The difference is that foldMapA only requires G to be an Applicative | ||
| * rather than a Monad. It is also slower due to use of Eval. |
There was a problem hiding this comment.
Do we actually know that this is true? My intuition is that it generally will be, but there's also some Either overhead involved in the tailRecM implementation.
|
I put together a little benchmark with a vector with 10k elements with examples that both short-circuit in |
|
Let's update the scaladoc and get this merged :) |
|
@LukaJCB I'm not quite sure what more I'm supposed to add for scaladoc. Isn't it enough to say that this does the same thing as |
|
Sorry had a brainfart there 😄 |
mberndt123
changed the title
Applicative is enough for this method, so it shouldn't require Monad.